feat(csp): wire report-only CSP to a real reporting endpoint#263
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds a same-origin CSP reporting endpoint at ChangesCSP Reporting
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/app/api/csp-report/route.test.ts (1)
120-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest only exercises the spoofable declared-length shortcut, not the real byte cap.
This test sends a small body with a large declared
content-lengthheader, which only proves the early declared-length check works. It doesn't cover the case of an actual oversized body without a (or with an honest small)content-lengthheader, which is exactly the path that currently buffers the whole body viarequest.text()before checking size (see comment onroute.ts). Consider adding a test that sends a real >8KB body to confirm the post-read cap actually works as intended.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/csp-report/route.test.ts` around lines 120 - 129, The current csp-report test only covers the spoofable declared content-length shortcut, so add coverage in the POST test suite for the real byte-size limit path in POST by sending an actually oversized request body and verifying it is dropped after read. Update the test around POST in csp-report/route.test.ts to use a payload larger than the 8KB cap with an honest or absent content-length header, so it exercises the request.text() buffering path and the post-read size check instead of only the declared-length branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/api/csp-report/route.ts`:
- Around line 84-99: The POST handler in POST currently buffers the full request
body with request.text() before enforcing MAX_BODY_BYTES, so oversized payloads
can still consume memory. Update the body handling in
src/app/api/csp-report/route.ts to read the request as a stream, count bytes
incrementally, and abort immediately once the limit is exceeded; keep the
existing noContent behavior and preserve the limiter/clientIp/extractViolations
flow around the new streaming read.
---
Nitpick comments:
In `@src/app/api/csp-report/route.test.ts`:
- Around line 120-129: The current csp-report test only covers the spoofable
declared content-length shortcut, so add coverage in the POST test suite for the
real byte-size limit path in POST by sending an actually oversized request body
and verifying it is dropped after read. Update the test around POST in
csp-report/route.test.ts to use a payload larger than the 8KB cap with an honest
or absent content-length header, so it exercises the request.text() buffering
path and the post-read size check instead of only the declared-length branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5f45c2b1-35ca-4b80-b75c-d19b70176380
📒 Files selected for processing (3)
next.config.tssrc/app/api/csp-report/route.test.tssrc/app/api/csp-report/route.ts
|
🚀 Preview: https://wcpos-5i1j801yu-wcpos.vercel.app |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d6173cb23
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Post-fix triage for review feedback:
Skipped threads:
Validation:
Remaining unresolved inline review threads after resolving: 0. |
The Content-Security-Policy-Report-Only header had no report-to/report-uri directive, so browsers warned it 'will have no effect' — a report-only policy with nowhere to send reports neither blocks nor reports, making it inert (and noisy in the console). Add the reporting plumbing so the header does its job: - New POST /api/csp-report sink that accepts both wire formats (legacy application/csp-report and the Reporting API's application/reports+json), rate-limited + size-capped + always 204, logging via infraLogger (Loki/ Sentry, not the Discord sale path). - report-uri + report-to directives on the CSP, plus a Reporting-Endpoints header naming the csp-endpoint group. This finally makes the 'verify clean, then enforce' plan actionable: real violations now land in the log sink instead of being silently discarded.
kilbot
force-pushed
the
worktree-csp-reporting-endpoint
branch
from
db6356b to
82f8291
Compare
There was a problem hiding this comment.
kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
/fix triage — merge-readiness pass
Verification (post-rebase)
All 4 review threads resolved; the only blocker (E2E) was a stale-base artifact now cleared. Re-running CI to confirm green. |
|
✅ CI is green after the rebase — Test, E2E Tests, and both CodeQL analyses all pass. The earlier E2E failure ( |
|
🚀 Preview: https://wcpos-5jkukj16n-wcpos.vercel.app |
1 participant
Problem
The site was emitting this console error (twice — once per document, e.g. page + Stripe iframe) on every page load:
The
Content-Security-Policy-Report-Onlyheader innext.config.tshad noreport-to/report-uridirective and no collection endpoint anywhere in the app. A report-only policy never blocks (that's the point), so with nowhere to send reports it does nothing at all — the browser is correct that it "will have no effect." It's been inert this whole time, which also means the code comment's "verify clean against real traffic, then promote to enforcing" plan could never actually gather any data.Fix (best-practice reporting wiring)
Rather than delete the header or blindly enforce an unvalidated policy, make the report-only phase actually work:
POST /api/csp-reportsink modeled on the existingreport-failureroute:application/csp-report(report-uri, used by Safari/Firefox) and the modernapplication/reports+jsonarray (report-to, Reporting API, Chromium).infraLogger.warn→ Loki/Sentry. Deliberately not the sale/Discord path: a CSP violation is diagnostic, not something to page anyone about.204— a broken/abusive report must never surface to the visitor.report-uri /api/csp-report(legacy fallback) andreport-to csp-endpoint(modern).Reporting-Endpointsheader: names thecsp-endpointgroup with the same-origin path, so it follows whichever host served the page (wcpos.com / www).Once real violations have been observed clean, the header can be flipped from
-Report-Onlyto enforcing.Verification
src/app/api/csp-report/route.test.ts): both formats, multi-violation arrays, non-CSP report filtering, log-injection sanitization, rate-limit drop, malformed JSON, oversized body. All green.tsc --noEmit: clean on changed files.next dev):Reporting-Endpoints: csp-endpoint="/api/csp-report"and the CSP ends with…; report-uri /api/csp-report; report-to csp-endpoint.POST /api/csp-reportreturns204for legacy, modern, and malformed bodies.wcpos·infra; malformed body logged nothing.Notes
report-failure(abuse cap + no raw logging of attacker-controlled input).Summary by CodeRabbit